-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Commentor #723
Commentor #723
Conversation
…pport multiple glob patterns
…che and documentor files
@@ -160,7 +160,7 @@ index N in the above snippets, and then be prefixed with exactly the same whites | |||
the original snippets above. See also the following examples of the expected response format. | |||
CHANGELOG: | |||
\`\`\` | |||
\`\`\`changelog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect code fence; 'changelog' language specifier should be removed.
generated by pr-docs-review-commit
incorrect_code_fence
@@ -217,9 +217,12 @@ export function applyLLMPatch(source: string, chunks: Chunk[]): string { | |||
const line = | |||
chunk.state === "deleted" ? undefined : chunk.lines[li] | |||
const linei = chunk.lineNumbers[li] - 1 | |||
if (isNaN(linei)) throw new DiffError("missing line number") | |||
if (isNaN(linei)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message thrown when the line number is NaN is not descriptive enough. Consider providing more context in the error message.
generated by pr-review-commit
missing_error_message
@@ -217,9 +217,12 @@ export function applyLLMPatch(source: string, chunks: Chunk[]): string { | |||
const line = | |||
chunk.state === "deleted" ? undefined : chunk.lines[li] | |||
const linei = chunk.lineNumbers[li] - 1 | |||
if (isNaN(linei)) throw new DiffError("missing line number") | |||
if (isNaN(linei)) | |||
throw new DiffError(`diff: missing or nan line number`) | |||
if (linei < 0 || linei >= lines.length) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message thrown when the line number is invalid is not descriptive enough. Consider providing more context in the error message.
generated by pr-review-commit
missing_error_message
return unique(systems.filter((s) => !!s)) | ||
} | ||
|
||
// Helper function to resolve tools in the project and return their system IDs | ||
// Finds systems in the project associated with a specific tool | ||
function resolveTools(prj: Project, tool: string): string[] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no error handling for the case where the tool is not found in the project. Consider adding error handling to provide a more robust solution.
generated by pr-review-commit
missing_error_handling
The changes in the pull request primarily involve commenting the code for better understanding. It also includes minor code improvements such as:
All these changes seem to improve the flexibility, readability and reliability of the code without introducing any apparent functional issues. So, my response is: LGTM 🚀
|
@@ -28,20 +46,33 @@ export function renderMessageContent( | |||
| ChatCompletionToolMessageParam | |||
): string { | |||
const content = msg.content |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon
generated by pr-review-commit
missing_semi
throw new DiffError("invalid line number") | ||
throw new DiffError( | ||
`diff: invalid line number ${linei} in ${lines.length}` | ||
) | ||
lines[linei] = line | ||
} | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon
generated by pr-review-commit
missing_semi
}) | ||
return match | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon
generated by pr-review-commit
missing_semi
@@ -217,9 +217,12 @@ export function applyLLMPatch(source: string, chunks: Chunk[]): string { | |||
const line = | |||
chunk.state === "deleted" ? undefined : chunk.lines[li] | |||
const linei = chunk.lineNumbers[li] - 1 | |||
if (isNaN(linei)) throw new DiffError("missing line number") | |||
if (isNaN(linei)) | |||
throw new DiffError(`diff: missing or nan line number`) | |||
if (linei < 0 || linei >= lines.length) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling for potential failure of the minimatch
function.
generated by pr-review-commit
missing_error_handling
export function isGlobMatch(filename: string, patterns: string | string[]) { | ||
return arrayify(patterns).some((pattern) => { | ||
const match = minimatch(filename, pattern, { | ||
windowsPathsNoEscape: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling for potential failure of the minimatch
function.
generated by pr-review-commit
missing_error_handling
@@ -301,7 +300,11 @@ export function createChatGenerationContext( | |||
if (pattern) | |||
appendChild( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling for potential failure of the arrayify
function.
generated by pr-review-commit
missing_error_handling
…e counting.py comments
@@ -160,7 +160,7 @@ index N in the above snippets, and then be prefixed with exactly the same whites | |||
the original snippets above. See also the following examples of the expected response format. | |||
CHANGELOG: | |||
\`\`\` | |||
\`\`\`changelog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changelog format has been changed from a generic code block to a specific 'changelog' code block, which may not be recognized by the rendering engine or may alter the intended presentation.
generated by pr-docs-review-commit
invalid_changelog_format
|
||
## Full source ([GitHub](https://github.com/microsoft/genaiscript/blob/main/packages/vscode/genaisrc/cmt.genai.mts)) | ||
|
||
<Code code={source} wrap={true} lang="ts" title="cmt.genai.mts" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new file 'cmt.mdx' has been added. Ensure that all new content is accurate, all links are working, and it is correctly integrated into the documentation structure.
generated by pr-docs-review-commit
new_file
@@ -34,7 +34,7 @@ See [configuration](/genaiscript/getting-started/configuration). | |||
|
|||
## Files | |||
|
|||
`run` takes one or more [glob](https://en.wikipedia.org/wiki/Glob_(programming)) patterns to match files in the workspace. | |||
`run` takes one or more [glob](<https://en.wikipedia.org/wiki/Glob_(programming)>) patterns to match files in the workspace. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect link format, angle brackets should not be used in markdown links.
generated by pr-docs-review-commit
link_format
@@ -150,7 +150,7 @@ permissions: | |||
pull-requests: write | |||
``` | |||
|
|||
- set the `GITHUB_TOKEN` secret in the `env` when running the cli | |||
- set the `GITHUB_TOKEN` secret in the `env` when running the cli |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect indentation, there should be no leading spaces before the bullet point.
generated by pr-docs-review-commit
indentation
@@ -160,7 +160,7 @@ index N in the above snippets, and then be prefixed with exactly the same whites | |||
the original snippets above. See also the following examples of the expected response format. | |||
CHANGELOG: | |||
\`\`\` | |||
\`\`\`changelog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing language specifier for the code fence, it should specify the language for syntax highlighting.
generated by pr-docs-review-commit
code_fence_lang
import { Code } from "@astrojs/starlight/components" | ||
import source from "../../../../../../../packages/vscode/genaisrc/cmt.genai.mts?raw" | ||
|
||
As developers, we understand the importance of well-commented code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing URL for the hyperlink in the text "Inspired by [a tweet]".
generated by pr-docs-review-commit
missing_link
JSONLineCache
class incache.ts
was expanded with additional comments for better code understanding. 📝cancellation.ts
include remarks for constructors and methods to enhance clarity and maintainability of the code. ✅chatcache.ts
, making it simpler for other engineers to grasp the code's purpose and functionality. 👥changelog.test.ts
anddiff.test.ts
suggest an improvement in test coverage. 🧪glob.ts
, the functionisGlobMatch
now accepts an array of patterns for greater flexibility. ➕runpromptcontext.ts
was improved to accept an array of patterns forfileOutput
. 🎛systems.ts
, tools resolution was improved to return system IDs linked to specific tools. 🔧TOMLTryParse
function intoml.ts
now has better error handling to prevent crashes on parsing errors. 💪prompt_template.d.ts
andprompt_type.d.ts
, thedefFileOutput
function has been modified to accept either a string or an array of strings for pattern specification. 👌cmt.genai.mts
has been added, this is a script using generative AI to add comments to other source code files. 🤖